Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] move "progress" from bib to images for ibcli #1150

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Jan 17, 2025

This is a bit of a RFC - it would be nice to have ibcli use progress as bib does. This is pretty trivial and done in https://github.com/osbuild/image-builder-cli/pull/21/files - but it needs the "progress" library that is currently living in bib.

This PR is the most simple approach - just copy it over with its warts, including that there is a progress.RunOSBuild() that is slightly different from osbuild.RunOSBuild. I guss one coulld justify this by saying one is about progress and the other is not. Then we can have ibcli use it and get it with more and nicer features into fedora.

Alternatively we can do the cleanup of this first, the most important part to cleanup is the unification of the to way to run osbuild.

mvo5 added 11 commits January 17, 2025 13:03
This commit adds a progressbar interface and implementations:
1. terminalProgressBar based on github.com/cheggaaa/pb/v3
2. plainProgressBar to just pass the message but no progress
3. debugProgressBar to print the osbuild/images status

The interface is very tailored to the needs of bib but it should
be similar enough to be reusable by `image-builder-cli`.

It contains a top level spinner that gives the current step:
"Make manifest" or "Building image". The the subprogress from
osbuild: the pipelines and the stages from the pipelines as
progress bars and the last line is the latest status message,
usually something from osbuild like "Starting pipeline qcow2".

The spinenr is a bit of a creative choice, it could also be a
progress bar because we know the steps (manifest+build). But the
runtime is totally dominated by the build step so having a first level
progress bar is odd as the UI will spend 90% of the time in it.
Plus the spinner gives it more a more "dynamic" appearnce.
This commit implements enough of `pb.Pool` for our needs
without the need to implement raw terminal access.

It turns out that by default podman does not connect the
full tty, even in `--privileged` mode. This is a sensible
security default but it means `pb.Pool` does not work as
it wants to set the terminal into "raw" mode and will fail
with an ioctl() error when not running on a terminal.

However we really just need simple ANSI sequences to
render the pool so this seems unnecessary. The initial
idea was to just use  `--log-driver=passthrough-tty` but
that is not available on podman 4.x. (which is part of
Ubuntu 24.04 LTS and the GH actions).

So this commit just implemnts a custom pool like renderer.
The Start/Stop methods can no longer error so remove the error
from the signature. The commit also tweaks the doc strings and
comments slightly.
This commit adds some extra checks to ensure that the pb.ProgressBar
does not contain any errors. This should not happen but we want
to be paranoid (especially since the error handling of pb.ProgressBar
is a bit indirect).
This commit fixes a bug in the progress rendering when no terminal
width can be identified. In this case pb.ProgressBar will strip
bars with dynamic elements. This is undesirable so when no size
can be found we just use the pb.defaultBarWidth (which is not
exported sadly). This should get fixed upstream ideally.
This commit fixes the missing \n when plain progress is used. Without
the output looks rather silly :/
This commit adds automatic progress bar selection based on checking
if we run on a terminal or not. When running on a terminal we use
the nice "terminalProgressBar". If that is not set we assuem
we run in a script or CI/CD environment and select plainProgressBar.

Thanks Colin for the hint about the bad integration test.
Previously an empty string in `--progress` meant to "auto-select".
But that is not super intuitive so this commit makes it explicit
using "auto".

Thanks to Achilleas for the suggestion.
This commit renames the "plain" progress to "verbose" progress. The
new name is closer to what the intention of the this progress is
and easier for the users to understand.

Thanks to Ondrej for the suggestion.
This commit finishes moving the bootc-image-builder "progress"
package into "images". This allows code-sharing between bib
and ibcli.

Note that further work will happen to unify `osbuild.RunOsbuild`
and `progress.RunOSBuild()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant